core/tests/test_event_loop_node.py-872-878 (1)

872-878: ⚠️ Potential issue | 🟡 Minor

Assert the ordering that this regression is about.

These assertions only prove both events were emitted. If CLIENT_INPUT_REQUESTED fires before the streamed explanation, the test still passes.

🧪 Tighten the assertion

         assert output_events
         assert input_events
+        assert received.index(output_events[0]) < received.index(input_events[0])
         assert "Root cause: checkout requests are failing" in output_events[0].data["snapshot"]
         assert input_events[0].data["prompt"] == "What would you like to do next?"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/tests/test_event_loop_node.py` around lines 872 - 878, The test only
asserts both CLIENT_OUTPUT_DELTA and CLIENT_INPUT_REQUESTED exist but not their
ordering; update the test to locate the indices of the first CLIENT_OUTPUT_DELTA
and first CLIENT_INPUT_REQUESTED in received (e.g. using enumerate and next or a
helper) and assert that the CLIENT_OUTPUT_DELTA index is less than the
CLIENT_INPUT_REQUESTED index before keeping the existing checks on
output_events[0].data["snapshot"] and input_events[0].data["prompt"]; reference
the variables received, output_events, input_events and EventType to find and
compare their positions.

core/framework/server/app.py-58-63 (1)

58-63: ⚠️ Potential issue | 🟡 Minor

Trimmed input is checked, but untrimmed input is still resolved.

At Line 51-56 you sanitize agent_path into raw_path, but Line 58 uses Path(agent_path) instead of Path(raw_path). Inputs like " examples/some_agent " will still fail unexpectedly.

💡 Suggested fix

-    candidate = Path(agent_path).expanduser()
+    candidate = Path(raw_path).expanduser()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/server/app.py` around lines 58 - 63, The code constructs
candidate using Path(agent_path) but earlier sanitizes the user input into
raw_path, so untrimmed inputs like "  examples/some_agent  " bypass trimming;
change the candidate initialization to use Path(raw_path) (i.e., replace
Path(agent_path) with Path(raw_path)) so all subsequent resolution and
_REPO_ROOT joining operate on the trimmed/sanitized value (ensure this change is
applied where candidate is assigned and continues to be used).

core/tests/test_session_manager_worker_handoff.py-174-177 (1)

174-177: ⚠️ Potential issue | 🟡 Minor

Replace fixed sleep with deterministic await to avoid flaky CI.

Using await asyncio.sleep(0.05) at Line 174 makes this test timing-sensitive. Prefer waiting until the mocked coroutine is actually awaited (with timeout).

💡 Suggested fix

-    await asyncio.sleep(0.05)
+    async def _wait_for_consolidate() -> None:
+        while consolidate.await_count == 0:
+            await asyncio.sleep(0)
+
+    await asyncio.wait_for(_wait_for_consolidate(), timeout=1.0)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/tests/test_session_manager_worker_handoff.py` around lines 174 - 177,
Replace the flaky fixed sleep with a deterministic wait that times out: instead
of await asyncio.sleep(0.05), use asyncio.wait_for with a short polling loop
that checks the AsyncMock state until consolidate has been awaited (e.g., poll
consolidate.await_count or consolidate.awaited flag) and raise on timeout; then
run consolidate.assert_awaited_once() and assert
queen_node.inject_event.await_count == 0. Target the existing mock names
(consolidate and queen_node.inject_event) so the test reliably waits for the
mocked coroutine to be awaited rather than sleeping.

core/framework/runtime/event_bus.py-538-539 (1)

538-539: ⚠️ Potential issue | 🟡 Minor

Keep the traceback when a handler fails.

Line 539 downgrades this to a plain message, but these exceptions are otherwise swallowed by gather(..., return_exceptions=True). Without exc_info, this becomes the only place we could have captured the failing handler's stack trace.

Suggested fix

-                except Exception as e:
-                    logger.error(f"Handler error for {event.type}: {e}")
+                except Exception:
+                    logger.exception("Handler error for %s", event.type)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/runtime/event_bus.py` around lines 538 - 539, The except block
that currently does logger.error(f"Handler error for {event.type}: {e}") should
preserve the stack trace; update the handler exception logging (in the event
dispatch/handler loop in event_bus.py where the except Exception as e block is)
to include the exception info by using logger.error(..., exc_info=True) or
logger.exception(...) so the traceback is retained when a handler fails.

core/tests/test_codex_planning_phase.py-105-111 (1)

105-111: ⚠️ Potential issue | 🟡 Minor

Avoid sleeping to decide when the node is safe to stop.

Line 106 makes this test timing-sensitive: on a slow runner the block event may not have fired yet, and on a fast runner an extra loop can already start. Waiting on the actual signal is more stable.

Suggested fix

     async def shutdown_after_first_block() -> None:
-        await asyncio.sleep(0.05)
+        await asyncio.wait_for(blocked.wait(), timeout=1.0)
         node.signal_shutdown()

Add a shared event next to received, and set it from capture():

blocked = asyncio.Event()

async def capture(event: AgentEvent) -> None:
    received.append(event)
    if _client_input_counts_as_planning_ask(event):
        phase_state.planning_ask_rounds += 1
    blocked.set()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/tests/test_codex_planning_phase.py` around lines 105 - 111, The test is
timing-sensitive because shutdown_after_first_block uses asyncio.sleep; instead
create a shared asyncio.Event (e.g., blocked) alongside received, set
blocked.set() inside the capture(event: AgentEvent) callback after updating
received and phase_state.planning_ask_rounds (and after calling
_client_input_counts_as_planning_ask), and replace the sleep/wait logic in
shutdown_after_first_block with awaiting blocked.wait() before calling
node.signal_shutdown(); keep references to shutdown_after_first_block,
node.execute(ctx), capture, received, phase_state.planning_ask_rounds and
_client_input_counts_as_planning_ask to locate and update the code accordingly.

core/tests/test_session_manager_worker_validation.py (1)

78-81: Avoid hard-coding subprocess arg positions in this assertion.

Lines 78-81 couple the test to the current argv layout instead of the behavior being verified. A harmless refactor of _run_validation_report_sync() can reshuffle the command and break this test even if it still invokes the right validator.

Suggested fix

-    script = captured["cmd"][4]
+    script = next(part for part in captured["cmd"] if "_validate_agent_package_impl" in str(part))
     assert "_validate_agent_package_impl" in script
     assert "validate_agent_package(agent_name)" not in script
-    assert captured["cmd"][6] == "/tmp/demo_agent"
+    assert any(part == "/tmp/demo_agent" for part in captured["cmd"])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/tests/test_session_manager_worker_validation.py` around lines 78 - 81,
The test currently hard-codes subprocess argument positions in assertions
against captured["cmd"]; instead, search the cmd list for the validator script
name and assert presence and correct surrounding args without relying on fixed
indices. Concretely, in the test that exercises _run_validation_report_sync(),
replace assertions like captured["cmd"][4] and captured["cmd"][6] with: assert
any(arg for arg in captured["cmd"] if "_validate_agent_package_impl" in arg),
then locate that element's index via captured["cmd"].index(...) and assert the
next/expected argument equals "/tmp/demo_agent" (and assert that the literal
"validate_agent_package(agent_name)" is not present anywhere using not
any(...)). This makes the test robust to reordering while still verifying the
validator is invoked with the correct agent path.

core/framework/server/session_manager.py (1)

211-250: Persist queen-local subscription IDs before parking.

suspend_queen() only removes the subscriptions stored on Session, but create_queen() also registers queen-local listeners on the shared EventBus. Without persisting those IDs, every park/revive cycle leaks another stale listener because this method has no way to unsubscribe them here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/server/session_manager.py` around lines 211 - 250,
suspend_queen currently only clears Session.worker_handoff_sub and
Session.memory_consolidation_sub but does not remove queen-local listeners that
create_queen registers on the shared EventBus, causing listener leaks; fix by
having create_queen store the subscription IDs it registers into a new Session
attribute (e.g., session.queen_local_subs or similar) and then modify
suspend_queen to iterate over session.queen_local_subs, call
session.event_bus.unsubscribe(...) for each saved id (with the same try/except
pattern used for other subs), and then clear session.queen_local_subs (set to
None or empty) along with the other session fields so all queen-local listeners
are unsubscribed when parking.

core/framework/tools/queen_lifecycle_tools.py (1)

1059-1078: Resolve the same entry point you trigger later.

This helper assumes entry_points[0] is the default entry, but the execution paths below all call runtime.trigger(entry_point_id="default", ...). Looking up that same entry spec here would make input shaping and reruns safe for multi-entry agents.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/tools/queen_lifecycle_tools.py` around lines 1059 - 1078, The
helper _get_default_entry_input_keys currently picks entry_points[0] but
runtime.trigger uses entry_point_id="default", so change the lookup to find the
entry spec whose identifier/name equals "default" (e.g., check attributes like
id or name on each spec) and fall back to the first spec if not found; then
derive entry_node_id from that matched entry_spec (or graph.entry_node if
missing) and use graph.get_node to return its input_keys (preserving existing
None/empty guards). This ensures _get_default_entry_input_keys resolves the same
entry point used by runtime.trigger.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

core/framework/tools/queen_lifecycle_tools.py (1)

987-999: ⚠️ Potential issue | 🟠 Major

Stop falling back to unrelated sessions when session_id is known.

If a specific session_id is supplied but its state.json does not exist yet, this still falls back to session_*/state.json and can borrow another run's defaults. That reintroduces cross-session input bleed on fresh runs/reruns.

Suggested fix

     if session_id:
         state_path = sessions_dir / session_id / "state.json"
         candidate_state_paths = [state_path] if state_path.exists() else []
     else:
         candidate_state_paths = []

-    if not candidate_state_paths:
+    if session_id and not candidate_state_paths:
+        return {}
+
+    if not candidate_state_paths:
         candidate_state_paths = sorted(
             sessions_dir.glob("session_*/state.json"),
             key=lambda path: path.stat().st_mtime if path.exists() else 0,
             reverse=True,
         )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/tools/queen_lifecycle_tools.py` around lines 987 - 999, When a
specific session_id is given the code currently falls back to other sessions'
state.json if that session's state file is missing; stop that by only using the
glob-based fallback when no session_id was provided. Change the fallback
condition so the sorted(sessions_dir.glob("session_*/state.json")) assignment
runs only when session_id is falsy (e.g. replace "if not candidate_state_paths:"
with "if not candidate_state_paths and not session_id:" or equivalent). This
touches the candidate_state_paths variable and the session_id/state.json logic
in queen_lifecycle_tools.py (look for the session_id block and the later
fallback that calls sessions_dir.glob).

core/frontend/src/lib/run-inputs.ts (1)

1-8: Consider extracting QueenPhase to a shared location.

The QueenPhase type is duplicated here and in chat-helpers.ts. If phases change, both need updates. A shared types.ts or barrel export would reduce drift.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/frontend/src/lib/run-inputs.ts` around lines 1 - 8, QueenPhase is
duplicated across modules; extract the type into a single shared module and
import it where needed. Create a shared types file (e.g., export type QueenPhase
= "planning" | "building" | "staging" | "running";), export the QueenPhase type,
then replace the inline declaration in run-inputs.ts (the QueenPhase type) and
the duplicate in chat-helpers.ts to import { QueenPhase } from that shared
module; ensure any existing references (e.g., RUNNABLE_PHASES or functions
expecting QueenPhase) still type-check after the import and update exports if
you need to re-export the type from a barrel.

core/framework/llm/litellm.py (2)

1696-1697: Remove duplicate import.

FinishEvent, TextDeltaEvent, and TextEndEvent are already imported at line 1684-1689 within this function.

♻️ Remove duplicate import

             if not output_text:
                 return []
-            from framework.llm.stream_events import FinishEvent, TextDeltaEvent, TextEndEvent
-
             usage = getattr(response, "usage", None)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/llm/litellm.py` around lines 1696 - 1697, Remove the duplicate
import of FinishEvent, TextDeltaEvent, and TextEndEvent: locate the redundant
"from framework.llm.stream_events import FinishEvent, TextDeltaEvent,
TextEndEvent" inside the same function (the earlier import at the top of that
function already brings these in) and delete this duplicate import so the three
symbols are only imported once.

---

663-668: Minor: redundant conditional branches for max_tokens.

Both branches of the if not stream / else set max_tokens identically. This could be simplified to a single unconditional assignment.

♻️ Suggested simplification

-        if not stream:
-            kwargs["max_tokens"] = max_tokens
-        else:
-            kwargs["max_tokens"] = max_tokens
-            if not self._is_anthropic_model():
-                kwargs["stream_options"] = {"include_usage": True}
+        kwargs["max_tokens"] = max_tokens
+        if stream and not self._is_anthropic_model():
+            kwargs["stream_options"] = {"include_usage": True}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/llm/litellm.py` around lines 663 - 668, The two-branch
conditional redundantly assigns kwargs["max_tokens"] in litellm.py; simplify by
assigning kwargs["max_tokens"] = max_tokens unconditionally, then keep the
stream-specific logic: if stream is truthy and not self._is_anthropic_model()
add kwargs["stream_options"] = {"include_usage": True}. Update the block around
the existing use of variables stream, max_tokens, kwargs and the call to
self._is_anthropic_model() in the same function to remove the unused if/else.

tools/coder_tools_server.py (2)

2073-2086: In-process import for behavior validation may leak module state across calls.

The behavior validation imports the agent module in-process (lines 2073-2086) after clearing stale entries from sys.modules. While the cache clearing helps, this approach differs from other validation steps that use subprocesses for isolation. If _validate_agent_package_impl is called multiple times in the same process with different agents, the sys.path modification (line 2076) accumulates paths.

This is likely acceptable for the current use case (subprocess-per-validation from session_manager), but consider documenting this limitation or using a subprocess for full isolation if the function is called in-process repeatedly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/coder_tools_server.py` around lines 2073 - 2086, The code in
_validate_agent_package_impl modifies sys.path by inserting package_parent and
clears sys.modules before importlib.import_module(package_name), which can
accumulate package_parent entries if called multiple times; update the function
to either (a) restore sys.path after the import (remove the inserted
package_parent if you added it) to avoid path accumulation and ensure you only
clear the specific sys.modules entries you previously removed, or (b) switch
this validation path to run in a subprocess for full isolation (matching other
validators); also add a short docstring/note on _validate_agent_package_impl
documenting the in-process limitation if you keep the current approach.

---

477-486: Entry-node intake detection may have false positives for legitimate parsing nodes.

The heuristic at lines 477-486 flags entry nodes that look like "intake/config parsers" based on keyword matching. However, agents that legitimately need to validate/normalize input before doing real work may get blocked. The condition combines intake_like and not direct_work_like with other signals, but terms like "validate", "normalize", "config", "path" (lines 463-475) are common in legitimate work nodes too.

Consider documenting the escape hatch (e.g., adding a direct-work hint to the node description) or relaxing the blocking behavior to a warning for edge cases where real work nodes happen to mention validation.

core/framework/server/tests/test_api.py (4)

663-691: Make terminal-choice test fully deterministic and side-effect explicit.

Using free-form "No, stop here" may couple this unit test to fuzzy matching. Prefer the canonical option string and assert queen.inject_event is not called before queen is parked.

Suggested test hardening

 async def test_chat_done_for_now_parks_queen_without_new_followup(self):
@@
-        async with TestClient(TestServer(app)) as client:
+        queen_node = session.queen_executor.node_registry["queen"]
+        async with TestClient(TestServer(app)) as client:
             resp = await client.post(
                 "/api/sessions/test_agent/chat",
-                json={"message": "No, stop here"},
+                json={"message": "Done for now"},
             )
@@
-        queen_node = session.queen_executor
-        assert queen_node is None
+        queen_node.inject_event.assert_not_awaited()
+        assert session.queen_executor is None
         session.event_bus.emit_client_output_delta.assert_awaited_once()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/server/tests/test_api.py` around lines 663 - 691, The test
test_chat_done_for_now_parks_queen_without_new_followup should use the exact
terminal option string from the event history instead of free-form input and
explicitly assert queen.inject_event is not called before the queen is parked;
update the POST payload to use "Done for now" (matching the options list from
session.event_bus.get_history) and add an assertion that
session.queen_executor.inject_event was not awaited/called prior to verifying
queen_node is None, ensuring the test checks both deterministic input and the
absence of the side-effect on queen.inject_event.

---

931-943: Assert stop result payload, not just HTTP status.

The test name says stop behavior should continue despite validation failures; asserting stopped in response would validate the functional outcome too.

Suggested assertion addition

         async with TestClient(TestServer(app)) as client:
             resp = await client.post(
                 "/api/sessions/test_agent/stop",
                 json={"execution_id": "exec_abc"},
             )
             assert resp.status == 200
+            data = await resp.json()
+            assert data["stopped"] is True

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/server/tests/test_api.py` around lines 931 - 943, The test
test_stop_ignores_worker_validation_failure currently only asserts HTTP 200;
after posting to "/api/sessions/test_agent/stop" with execution_id "exec_abc"
using TestClient(TestServer(app)), parse the JSON response body and assert the
expected functional outcome (e.g., that the payload contains "stopped": true or
the appropriate field indicating the execution was stopped) so the test verifies
the stop result in addition to resp.status; update the assertion after the POST
accordingly and keep existing setup of session.worker_validation_failures and
session.worker_runtime._mock_streams["default"]._execution_tasks["exec_abc"]
intact.

---

867-891: Add a no-execution assertion for resume validation blocking.

Like /trigger, /resume should prove that runtime execution is not started when worker validation fails.

Suggested test hardening

 async def test_resume_blocks_invalid_loaded_worker(self, sample_session, tmp_agent_dir):
@@
         session = _make_session(tmp_dir=tmp_path / ".hive" / "agents" / agent_name)
+        session.worker_runtime.trigger = AsyncMock()
         session.worker_validation_report = {
             "valid": False,
             "steps": {"tool_validation": {"passed": False}},
         }
@@
             assert data["validation_failures"] == [
                 "tool_validation: missing tool execute_command_tool"
             ]
+        session.worker_runtime.trigger.assert_not_awaited()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/server/tests/test_api.py` around lines 867 - 891, The test
should also assert that runtime execution is not started when worker validation
fails: mock or spy the function that the resume endpoint would call to begin
execution (e.g., the Session.start_runtime or the app's runtime-start function
used by the "/api/sessions/{agent}/resume" handler) before making the POST, then
after the POST assert that this start function was never called (or that no
runtime task/process was created), in addition to the existing 409 and
validation failure assertions; update test_resume_blocks_invalid_loaded_worker
to include that no-start assertion.

---

588-606: Also assert trigger is not invoked on validation failure.

This test checks the 409 payload, but it should additionally guard against accidental runtime execution when validation fails.

Suggested test hardening

 async def test_trigger_blocks_invalid_loaded_worker(self):
     session = _make_session()
+    session.worker_runtime.trigger = AsyncMock()
     session.worker_validation_report = {
         "valid": False,
         "steps": {"behavior_validation": {"passed": False}},
     }
@@
             data = await resp.json()
             assert "failed validation" in data["error"]
             assert data["validation_failures"] == ["behavior_validation: placeholder prompt"]
+        session.worker_runtime.trigger.assert_not_awaited()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/server/tests/test_api.py` around lines 588 - 606, The test
test_trigger_blocks_invalid_loaded_worker should also assert that the runtime
trigger path is not executed when validation fails; add a spy/mock (e.g.,
unittest.mock.AsyncMock) on the method that the endpoint would call to run the
agent (attach to the session object used in the test — e.g., session.trigger or
the worker run method used by your codebase) before creating the app, perform
the POST as shown, then assert that the mocked method was not awaited/called;
keep the existing 409 and payload assertions and only add this non-invocation
check to prevent accidental runtime execution.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

core/tests/test_queen_lifecycle_validation.py (1)

596-601: Unused mock attribute can be removed for clarity.

_get_primary_session_state is mocked but never called since run_agent_with_input now passes session_state=None directly. The mock is harmless but misleading.

🔧 Remove unused mock

     runtime = SimpleNamespace(
         resume_timers=MagicMock(),
         trigger=AsyncMock(),
-        _get_primary_session_state=MagicMock(return_value={}),
         graph=SimpleNamespace(nodes=[]),
     )

And similarly at line 637:

     runtime = SimpleNamespace(
         resume_timers=MagicMock(),
         trigger=AsyncMock(return_value="exec-1"),
-        _get_primary_session_state=MagicMock(return_value={}),
         get_entry_points=lambda: [SimpleNamespace(id="default", entry_node="process")],

Also applies to: 637-637

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/tests/test_queen_lifecycle_validation.py` around lines 596 - 601, The
runtime fixture creation includes an unused mocked attribute
_get_primary_session_state which is no longer called because
run_agent_with_input passes session_state=None; remove the
_get_primary_session_state=MagicMock(...) entry from the SimpleNamespace
initialization in both places (the runtime declarations around the test at lines
~596 and ~637) so the runtime only contains resume_timers, trigger, and graph to
avoid misleading unused mocks.

core/framework/tools/queen_lifecycle_tools.py (2)

2995-3021: Judge pressure detection has a potential edge case with mixed actions.

The streak detection breaks on ACCEPT or any action not in _NON_ACCEPT_JUDGE_ACTIONS, which includes actions not in the expected set. If a new verdict action is added (e.g., "PAUSE"), it would break the streak even though it's not an ACCEPT.

This is likely acceptable since unknown actions are rare, but worth noting for future maintainability.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/tools/queen_lifecycle_tools.py` around lines 2995 - 3021, The
current _get_recent_judge_pressure streak loop breaks on any action not in
_NON_ACCEPT_JUDGE_ACTIONS which causes new/unknown non-ACCEPT actions (e.g.,
"PAUSE") to prematurely end the streak; change the loop in
_get_recent_judge_pressure to only break when action == "ACCEPT" and treat any
other non-empty action as part of the non-ACCEPT streak (append unknown actions
to streak so they count), keeping the existing compression step and final
threshold check; reference the function name _get_recent_judge_pressure and the
constant _NON_ACCEPT_JUDGE_ACTIONS when making this update.

---

1071-1108: Preflight timeout is appropriate but consider surfacing the warning.

The 15-second timeout with a warning log is reasonable. However, if preflight times out, the worker proceeds without credential validation or MCP resync. The queen won't know about this unless it checks logs.

Consider returning a status flag or including a warning in the trigger response so the queen can inform the user that preflight was incomplete.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/tools/queen_lifecycle_tools.py` around lines 1071 - 1108,
_change the _preflight_worker_run signature and behavior to surface when
preflight did not complete: have _preflight_worker_run return a bool (True =
preflight completed, False = preflight incomplete/timed out) and set a
short-lived flag on the session (e.g., session.preflight_incomplete_warning =
True) when the timeout path is hit so callers (the queen/trigger response) can
surface the warning; specifically, update async def
_preflight_worker_run(session, runtime, timeout_seconds) to return bool, set
cred_error handling and MCP resync as-is but in the except TimeoutError block
set session.preflight_incomplete_warning = True and return False, otherwise
return True when _preflight finishes successfully so callers can include the
warning in the trigger response.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

core/framework/llm/litellm.py (4)

1720-1720: Remove duplicate import.

FinishEvent, TextDeltaEvent, and TextEndEvent are already imported at Lines 1708-1713. This inner import is redundant.

Remove duplicate import

             if not output_text:
                 return []
-            from framework.llm.stream_events import FinishEvent, TextDeltaEvent, TextEndEvent
-
             usage = getattr(response, "usage", None)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/llm/litellm.py` at line 1720, There is a redundant inner
import of FinishEvent, TextDeltaEvent, and TextEndEvent in litellm.py; remove
the duplicated import statement (the one importing FinishEvent, TextDeltaEvent,
TextEndEvent from framework.llm.stream_events around the inner block) since
those symbols are already imported earlier (lines importing
FinishEvent/TextDeltaEvent/TextEndEvent), leaving only the original top-level
import intact.

---

685-690: Simplify redundant conditional.

Both branches set max_tokens identically. The stream_options logic can be expressed more concisely.

Simplify max_tokens and stream_options assignment

-        if not stream:
-            kwargs["max_tokens"] = max_tokens
-        else:
-            kwargs["max_tokens"] = max_tokens
-            if not self._is_anthropic_model():
-                kwargs["stream_options"] = {"include_usage": True}
+        kwargs["max_tokens"] = max_tokens
+        if stream and not self._is_anthropic_model():
+            kwargs["stream_options"] = {"include_usage": True}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/llm/litellm.py` around lines 685 - 690, The code sets
kwargs["max_tokens"] in both branches redundantly; simplify by setting
kwargs["max_tokens"] = max_tokens once, then only add kwargs["stream_options"] =
{"include_usage": True} when stream is truthy and self._is_anthropic_model() is
false. Locate the block around the use of stream, kwargs, and the call to
self._is_anthropic_model() in litellm.py and replace the two-branch assignment
with a single max_tokens assignment and a single conditional that adds
stream_options only for non-Anthropic models when streaming.

---

1-35: Missing from __future__ import annotations import.

The coding guidelines require from __future__ import annotations for modern type syntax. This file uses type hints extensively but lacks this import.

Add future annotations import

 """LiteLLM provider for pluggable multi-provider LLM support.
 ...
 """
 
+from __future__ import annotations
+
 import ast
 import asyncio

As per coding guidelines: "Use from __future__ import annotations for modern type syntax".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/llm/litellm.py` around lines 1 - 35, Add the future
annotations import as the very first statement in the module: include "from
__future__ import annotations" at the top of core/framework/llm/litellm.py
before any other imports so forward references and postponed evaluation of type
hints work correctly for functions/classes like LLMProvider, LLMResponse, Tool
and any async iterator uses; ensure it remains the first non-comment line in the
file.

---

551-553: _codex_backend is a snapshot, not a live alias.

_codex_adapter.enabled is a @property that dynamically evaluates is_codex_api_base(self._provider.api_base). However, Line 553 captures this as a one-time boolean value at construction. If api_base were ever modified post-init, _codex_backend would become stale while _codex_adapter.enabled would reflect the update.

If this is intentional for backward compatibility with callers that expect a simple bool, consider adding a brief comment clarifying the snapshot semantics.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/framework/llm/litellm.py` around lines 551 - 553, The field
_codex_backend is currently assigned a one-time boolean from
CodexResponsesAdapter(self).enabled which snapshots
is_codex_api_base(self._provider.api_base) at construction and will go stale if
api_base changes; either make _codex_backend a live proxy (replace the snapshot
with a property or lambda that returns self._codex_adapter.enabled) or, if
snapshot semantics are intentional for backward compatibility, add a short
clarifying comment next to the assignment explaining that _codex_backend is a
construction-time snapshot and will not reflect subsequent changes to
_codex_adapter.enabled/_provider.api_base.